feat(l1): implement debug_traceCall RPC endpoint#6695
Conversation
|
Lines of code reportTotal lines added: Detailed view |
🤖 Kimi Code ReviewThe PR implements Code Quality & Correctness
let mut env = env_from_generic(tx, block_header, db, vm_type)?;
env.block_gas_limit = i64::MAX as u64;
adjust_disabled_base_fee(&mut env);Consider extracting this into a
let default_block = BlockIdentifierOrHash::Identifier(BlockIdentifier::default());Verify Security & ConsensusState Isolation — The implementation correctly uses Timeout Protection — All three trace methods in Gas Price Handling — The TestingThe integration tests in
Test Helper Safety — DocumentationThe doc comment on Minor Suggestions
VerdictThe code is correct, secure, and well-tested. The implementation follows Ethereum RPC specifications appropriately and includes proper safeguards against resource exhaustion. No blocking issues identified. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
Assumption: Item 1 depends on the store retaining non-canonical blocks, but I couldn’t run the Rust tests here because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR implements the
Confidence Score: 4/5Safe to merge. The new endpoint follows established patterns for all three tracers and the test coverage is solid. Two minor code-quality notes but no functional defects. The implementation correctly threads through all layers and closely mirrors the existing transaction-tracer code paths. The only notable gap is that
|
| Filename | Overview |
|---|---|
| crates/vm/backends/levm/mod.rs | Exposes env_from_generic and adjust_disabled_base_fee as pub(crate), and adds vm_from_generic_with_tracer — a near-identical copy of vm_from_generic with an extra tracer parameter. The duplication is benign today but the original is not refactored to delegate. |
| crates/vm/backends/levm/tracing.rs | Adds three trace_call_* methods on LEVM that mirror the existing trace_tx_* methods. Pattern is consistent: build env, override block_gas_limit to i64::MAX, build VM with tracer, execute, return result. Logic is sound. |
| crates/blockchain/tracing.rs | Adds three async wrapper methods that follow the exact same timeout_trace_operation pattern used by the existing transaction tracers. No issues found. |
| crates/networking/rpc/tracing.rs | New TraceCallRequest implements parse/handle following the existing TraceTransactionRequest pattern. The reexec field from TraceConfig is silently ignored without documentation. Good inline unit tests added. |
| crates/networking/rpc/rpc.rs | One-line addition routing debug_traceCall to TraceCallRequest::call. Correct placement in the debug namespace dispatcher. |
| crates/vm/tracing.rs | Adds three thin delegating methods on Evm that forward to LEVM::trace_call_*. Correctly plumbed with vm_type and crypto passthrough. |
| test/tests/rpc/debug_trace_call_tests.rs | Good coverage: default tracer, omitted block param, by-block-number, by-block-hash (EIP-1898), prestateTracer, opcodeTracer, unknown block error, and empty-params error. |
| test/tests/rpc/helpers.rs | New shared test helpers file: setup_store, setup_genesis_only, setup_single_transfer_block, rpc_call, and rpc_call_expect_err. Well-structured and reusable. |
Sequence Diagram
sequenceDiagram
participant Client
participant RPC as TraceCallRequest (rpc/tracing.rs)
participant BC as Blockchain (blockchain/tracing.rs)
participant EVM as Evm (vm/tracing.rs)
participant LEVM as LEVM (vm/backends/levm/tracing.rs)
participant VM as VM (levm)
Client->>RPC: debug_traceCall([callObj, blockId?, traceConfig?])
RPC->>RPC: parse() — GenericTransaction + BlockIdentifierOrHash + TraceConfig
RPC->>BC: resolve block header
BC-->>RPC: BlockHeader
RPC->>BC: "trace_call_calls/prestate/opcodes(&header, &tx, timeout, ...)"
BC->>BC: StoreVmDatabase::new(storage, header)
BC->>BC: new_evm(vm_db)
BC->>BC: timeout_trace_operation(timeout, closure)
BC->>EVM: "trace_call_calls/prestate/opcodes(&header, &tx, ...)"
EVM->>LEVM: trace_call_calls/prestate/opcodes(db, header, tx, ...)
LEVM->>LEVM: env_from_generic(tx, header, db)
LEVM->>LEVM: "env.block_gas_limit = i64::MAX"
LEVM->>LEVM: "adjust_disabled_base_fee(&mut env)"
LEVM->>LEVM: vm_from_generic_with_tracer(tx, env, db, tracer)
LEVM->>VM: VM::new(env, db, tx, tracer, vm_type, crypto)
VM-->>LEVM: vm
LEVM->>VM: vm.execute()
VM-->>LEVM: Ok(())
LEVM->>VM: get_trace_result / opcode_tracer.take_result
VM-->>LEVM: CallTrace / PrestateResult / OpcodeTraceResult
LEVM-->>EVM: Result
EVM-->>BC: Result
BC-->>RPC: Result
RPC->>RPC: serialize result
RPC-->>Client: JSON response
Comments Outside Diff (1)
-
crates/vm/backends/levm/mod.rs, line 2838-2881 (link)vm_from_genericis now an exact duplicate ofvm_from_generic_with_tracerexcept for the hardcodedLevmCallTracer::disabled(). If the Transaction-building logic ever diverges (e.g. a new tx type is added), one copy will silently lag behind the other. The original can simply delegate to eliminate the duplication.Prompt To Fix With AI
This is a comment left during a code review. Path: crates/vm/backends/levm/mod.rs Line: 2838-2881 Comment: `vm_from_generic` is now an exact duplicate of `vm_from_generic_with_tracer` except for the hardcoded `LevmCallTracer::disabled()`. If the Transaction-building logic ever diverges (e.g. a new tx type is added), one copy will silently lag behind the other. The original can simply delegate to eliminate the duplication. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
crates/vm/backends/levm/mod.rs:2838-2881
`vm_from_generic` is now an exact duplicate of `vm_from_generic_with_tracer` except for the hardcoded `LevmCallTracer::disabled()`. If the Transaction-building logic ever diverges (e.g. a new tx type is added), one copy will silently lag behind the other. The original can simply delegate to eliminate the duplication.
```suggestion
fn vm_from_generic<'a>(
tx: &GenericTransaction,
env: Environment,
db: &'a mut GeneralizedDatabase,
vm_type: VMType,
crypto: &'a dyn Crypto,
) -> Result<VM<'a>, VMError> {
vm_from_generic_with_tracer(tx, env, db, vm_type, crypto, LevmCallTracer::disabled())
}
```
### Issue 2 of 2
crates/networking/rpc/tracing.rs:43-46
The `reexec` field is deserialized from the caller's `traceConfig` but is never read in `TraceCallRequest::handle`. For `debug_traceTransaction` and `debug_traceBlockByNumber`, `reexec` controls how far back to re-execute blocks to rebuild missing trie state. That concept doesn't apply here (state comes directly from the block header's state root), so ignoring it is correct — but unlike `stateOverrides`/`blockOverrides`, this divergence is not mentioned in the doc comment, which may surprise callers who pass it expecting an effect.
```suggestion
/// **Known divergence from geth**: geth's `traceConfig` accepts
/// `stateOverrides` and `blockOverrides` for hypothetical-state debugging.
/// ethrex does not yet honour either — they are silently ignored. Pass-through
/// support requires applying overrides at the VM layer before execution.
///
/// The `reexec` field in `traceConfig` is also accepted but has no effect:
/// state is read directly from the block header's state root rather than
/// being rebuilt by re-executing ancestor blocks.
```
Reviews (1): Last reviewed commit: "fix(rpc): broaden debug_traceCall test c..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// **Known divergence from geth**: geth's `traceConfig` accepts | ||
| /// `stateOverrides` and `blockOverrides` for hypothetical-state debugging. | ||
| /// ethrex does not yet honour either — they are silently ignored. Pass-through | ||
| /// support requires applying overrides at the VM layer before execution. |
There was a problem hiding this comment.
reexec field is deserialized from the caller's traceConfig but is never read in TraceCallRequest::handle. For debug_traceTransaction and debug_traceBlockByNumber, reexec controls how far back to re-execute blocks to rebuild missing trie state. That concept doesn't apply here (state comes directly from the block header's state root), so ignoring it is correct — but unlike stateOverrides/blockOverrides, this divergence is not mentioned in the doc comment, which may surprise callers who pass it expecting an effect.
| /// **Known divergence from geth**: geth's `traceConfig` accepts | |
| /// `stateOverrides` and `blockOverrides` for hypothetical-state debugging. | |
| /// ethrex does not yet honour either — they are silently ignored. Pass-through | |
| /// support requires applying overrides at the VM layer before execution. | |
| /// **Known divergence from geth**: geth's `traceConfig` accepts | |
| /// `stateOverrides` and `blockOverrides` for hypothetical-state debugging. | |
| /// ethrex does not yet honour either — they are silently ignored. Pass-through | |
| /// support requires applying overrides at the VM layer before execution. | |
| /// | |
| /// The `reexec` field in `traceConfig` is also accepted but has no effect: | |
| /// state is read directly from the block header's state root rather than | |
| /// being rebuilt by re-executing ancestor blocks. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 43-46
Comment:
The `reexec` field is deserialized from the caller's `traceConfig` but is never read in `TraceCallRequest::handle`. For `debug_traceTransaction` and `debug_traceBlockByNumber`, `reexec` controls how far back to re-execute blocks to rebuild missing trie state. That concept doesn't apply here (state comes directly from the block header's state root), so ignoring it is correct — but unlike `stateOverrides`/`blockOverrides`, this divergence is not mentioned in the doc comment, which may surprise callers who pass it expecting an effect.
```suggestion
/// **Known divergence from geth**: geth's `traceConfig` accepts
/// `stateOverrides` and `blockOverrides` for hypothetical-state debugging.
/// ethrex does not yet honour either — they are silently ignored. Pass-through
/// support requires applying overrides at the VM layer before execution.
///
/// The `reexec` field in `traceConfig` is also accepted but has no effect:
/// state is read directly from the block header's state root rather than
/// being rebuilt by re-executing ancestor blocks.
```
How can I resolve this? If you propose a fix, please make it concise.
🤖 Claude Code ReviewNow I have everything needed for a thorough review. Let me write it up. Review:
|
Add debug_traceCall which traces an arbitrary call (GenericTransaction) at a given block state without creating a transaction on-chain. Supports all existing tracers: callTracer, prestateTracer, opcodeTracer. Adds trace-call infrastructure at each layer: - LEVM: trace_call_calls, trace_call_prestate, trace_call_opcodes - Evm: wrapper methods for generic-tx tracing - Blockchain: async trace_call_* methods with timeout support - RPC: TraceCallRequest handler with geth-compatible param parsing Part of #6572
Set up a funded account, then call debug_traceCall with a simulated transfer. Assert the callTracer response shape.
Tests as shipped exercised one path: callTracer, "latest" block, against a genesis-only state. That left the three tracer variants asymmetric on coverage and didn't validate any of the items on the PR's own test plan. Added integration tests covering: prestateTracer + opcodeTracer paths, block-by-number, block-by-hash (EIP-1898 form), default-to-latest when the block parameter is omitted, unknown-block error, and empty-params error. The post-block tests pin `nonce` in the call object because ethrex's LEVM enforces nonce equality even on simulated calls — a pre-existing constraint shared with `eth_call`, documented inline so the next person doesn't repeat the diagnosis. Documented two ergonomic gaps on `TraceCallRequest` itself: - `stateOverrides` / `blockOverrides` from the third parameter are silently ignored. Real geth-compat support requires plumbing overrides down to the VM, so this is called out as a known divergence rather than fixed in-flight. - Pruned nodes that don't have the target block's state will surface the storage layer's "state root missing" error as a generic Internal. Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs` into the shared `rpc::helpers` module introduced in #6701; the file is renamed to `debug_trace_call_tests.rs`. Also polished the handler's default-block fallback (one allocation instead of an `Option::clone`) and switched the not-found error from "Block not Found" to "Block not found" to match the casing every other endpoint uses.
azteca1998
force-pushed
the
feat/debug-trace-call
branch
from
1ba1c02 to
04bce6e
Compare
State is sourced from the block header's state root, so the reexec field (which controls ancestor re-execution depth) is accepted but silently ignored.
2 participants
Summary
debug_traceCallwhich traces an arbitrary call at a given block state without creating a transaction on-chaincallTracer,prestateTracer,opcodeTracereth_callplus an optional trace configCloses part of #6572
Test plan
debug_traceCallwith callTracer returns nested call framesdebug_traceCallwith prestateTracer returns pre/post statedebug_traceCallwith opcodeTracer returns EIP-3155 step traceCloses #6641